Skip to content

fix(acp): publish current context and compression depth - #74032

Open
stefanpieter wants to merge 1 commit into
NousResearch:mainfrom
stefanpieter:fix/acp-current-context-compression-telemetry
Open

fix(acp): publish current context and compression depth#74032
stefanpieter wants to merge 1 commit into
NousResearch:mainfrom
stefanpieter:fix/acp-current-context-compression-telemetry

Conversation

@stefanpieter

@stefanpieter stefanpieter commented Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown

What does this PR do?

Make ACP context telemetry represent the current effective model window and durably expose completed compression depth.

ACP usage_update.used now prefers provider-reported last_prompt_tokens and uses the existing request estimator only before real model usage is available. Each completed worker-thread model step schedules a fresh update, including non-negative _meta.hermes.compressionCount.

Compression count round-trips through session storage, survives restore and compression-driven internal rotation, and is cleared durably by an explicit same-session ACP /reset.

Restored sessions replay existing history and then await the current usage/compression update before either session/load or session/resume returns, so a fresh client is current at the response boundary.

This complements #70267: that PR changes PromptResponse.usage.inputTokens; this PR fixes the authoritative ACP usage_update stream, compression metadata, and durable reset/rotation behavior.

Related Issue

Related to #70204 and #70267.

Changes made

  • acp_adapter/server.py: publish current-window usage/compression depth after model steps and during load/resume restore; persist cleared count on ACP /reset
  • agent/context_compressor.py: load, persist, rotate, and explicitly reset compression depth
  • hermes_state_common.py and hermes_state.py: add the authoritative schema column plus bounded session accessors after current main moved SCHEMA_SQL into the common module
  • regressions cover current-window usage, estimate fallback, mid-turn scheduling, metadata, load/resume response boundaries, rotation, explicit reset, and durable state
  • contributor attribution remains mapped by the repository helper

Review remediation

  • replaced deferred resume telemetry scheduling with an awaited update after history replay, matching the existing load response-boundary contract
  • added an exact resume regression that failed on the prior head because no UsageUpdate was visible when resume_session() returned
  • reconciled current-main schema/test splits without resurrecting obsolete inline schema or removed test blocks

Current exact-head verification

  • head: 1e0784da1333b06370567fa77ca59fed86da4843
  • scripts/run_tests.sh tests/acp/test_server.py tests/agent/test_context_engine_host_contract.py tests/test_hermes_state.py -q — 180 passed
  • Ruff, compileall, git diff --check, and added-line security scan — passed
  • independent exact-SHA review — PASS
  • branch contains reviewed base f3cda0ceb18d8ba7465a6d223098ef0e56c8fee1; live main later advanced only in tools/file_operations.py and its test, with no dependency overlap and a clean git merge-tree proof
  • upstream CI is awaiting external-fork workflow approval: https://github.com/NousResearch/hermes-agent/actions/runs/30619238988

Type of change

  • Bug fix (non-breaking)
  • Tests

Checklist

  • Contributing guide reviewed
  • Conventional commits
  • Focused regressions added
  • Tested on macOS
  • Full repository suite not claimed; affected hermetic suites are reported above

@stefanpieter

stefanpieter commented Jul 29, 2026

Copy link
Copy Markdown
Author

CI is awaiting the repository-admin approval required for workflows from external forks (run 30442121432). The submitter approval endpoint returns HTTP 403 (Must have admin rights to Repository).

Exact head eb05e60e3e31984efa5f1627ded239278bbef667 passed locally:

  • affected ACP/state suites: 589 passed
  • full tests/acp/: 332 passed
  • Ruff, compileall, and the 853-file Windows-footgun scan
  • independent exact-tree review: passed with no security or logic findings

@alt-glitch alt-glitch added type/feature New feature or request P4 Best-effort: we will get to it when we get to it (no commitment) comp/acp Agent Communication Protocol adapter comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/compression Context compression and continuation sessions area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 29, 2026
@stefanpieter

stefanpieter commented Jul 29, 2026

Copy link
Copy Markdown
Author

Post-submit live validation found and fixed a restore-only delivery gap: session/load had deferred usage_update, leaving a fresh client stale until its first prompt. The updated head awaits the restored update before returning.

Fresh evidence on the updated exact tree:

  • 590 affected/state tests passed
  • 333 full ACP tests passed
  • Ruff, compileall, and 853-file Windows footgun scan passed
  • real ACP wire probe emitted used=182079, size=272000, compressionCount=2 at message 305 before the load response at message 307
  • independent final review: passed, no findings

@stefanpieter
stefanpieter force-pushed the fix/acp-current-context-compression-telemetry branch from f709276 to f001538 Compare July 29, 2026 12:04
@stefanpieter stefanpieter mentioned this pull request Jul 29, 2026
Closed
@stefanpieter

stefanpieter commented Jul 29, 2026

Copy link
Copy Markdown
Author

@kshitijk4poor this is ready for maintainer action on exact head f0015386044fbc3191858aea4701faba4f0a3eef.

Could you please approve the external-fork workflow and review/merge if satisfied? The PR body has the exact base/head, patch hash, local evidence, and live ACP wire proof.

teknium1
teknium1 reviewed Jul 30, 2026
View reviewed changes

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the ACP usage stream through provider usage, restore delivery, and durable compression state. The current-main premise is real: acp_adapter/server.py:857-867 still prefers the rough estimate and load_session defers its usage update.

Problems

  • acp_adapter/server.py:1431 makes session/load publish synchronously, but resume_session has the same in-call replay contract at acp_adapter/server.py:1453-1455 and still defers its usage update at acp_adapter/server.py:1466. A resumed client can therefore retain the stale-telemetry behavior this change fixes for loads.

Suggested changes

  • Apply the awaited usage publication to resume_session and add a response-boundary regression alongside the new load test.
  • Current main moved SCHEMA_SQL to hermes_state_common.py:135 in 21c7ae8563; preserve the declarative column addition there during salvage.

Automated hermes-sweeper review.

Comment thread acp_adapter/server.py
# request lifetime; deferring it until after return can lose the update
# during transport/task handoff and leaves a resumed client stale until
# its first prompt.
await self._send_usage_update(state)

@teknium1 teknium1 Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resume_session has the same before-response replay contract at lines 1453-1455, but it still defers _schedule_usage_update(state) at line 1466. Please await _send_usage_update(state) there too and add a response-boundary regression; otherwise resumed clients can still miss the restored telemetry this change guarantees for session/load.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@stefanpieter

stefanpieter commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Resolved the current review blocker on 1e0784da1333b06370567fa77ca59fed86da4843.

  • session/resume now replays history and awaits the restored usage/compression update before returning, matching session/load
  • added the exact response-boundary regression that failed on the prior head
  • forward-ported durable compression-count state to current main's authoritative hermes_state_common.py schema without resurrecting removed schema/test blocks

Verification:

  • scripts/run_tests.sh tests/acp/test_server.py tests/agent/test_context_engine_host_contract.py tests/test_hermes_state.py -q — 180 passed
  • Ruff, compileall, diff check, added-line security scan — passed
  • independent exact-SHA review — PASS
  • remote branch and PR head both match the SHA above

The exact-head upstream CI run is awaiting maintainer approval for external-fork workflows: https://github.com/NousResearch/hermes-agent/actions/runs/30619238988

@digironin @teknium1 please re-review when available. Formal RequestReviewsByLogin is unavailable to this fork author; the historical gate label and unresolved review thread remain reviewer-controlled.

Stefan van Biljon
fix(acp): publish current context compression telemetry
42040c7 
Reimplement NousResearch#74032 on current main without retaining historical merge commits. Persist compression counts, restore them before load/resume usage updates, and publish ACP-native context telemetry.\n\nIncludes contributor mapping from the original branch.
@stefanpieter
stefanpieter force-pushed the fix/acp-current-context-compression-telemetry branch from 1e0784d to 42040c7 Compare August 1, 2026 13:30
This was referenced Aug 1, 2026
Open
Closed
@JoannaHoly

JoannaHoly commented Sep 25, 2026

Copy link
Copy Markdown

A real-world case for this PR, from a VS Code ACP client with gpt-6-astra on openai-codex.

After a native Responses compaction (Native Responses compaction item captured), usage_update.used kept estimating over the full 279-message history and read ~88–91% of 272,000. Meanwhile agent.log's own API call lines showed a provider-reported prompt of 180,234 tokens (66%). The request no longer carries the pre-checkpoint history, so the rough estimate stays well above the real prompt.

It is not the client: it renders used / size from Hermes' usage_update unchanged. Running estimate_request_tokens_rough over the same restored history gives the same figure (~215k for the messages alone, plus the system prompt and tool schemas).

Preferring last_prompt_tokens, as this PR does, would fix it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

Labels

area/compression Context compression and continuation sessions area/sessions Session lifecycle, resume, persistence, history comp/acp Agent Communication Protocol adapter comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P4 Best-effort: we will get to it when we get to it (no commitment) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@stefanpieter @JoannaHoly @teknium1 @alt-glitch